Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'load directly' mode to default Cucumber test suite #6664

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

mjjbell
Copy link
Member

@mjjbell mjjbell commented Jul 29, 2023

Issue

Currently, npm test runs the Cucumber suite with a configuration matrix for the algorithm (CH, MLD) and data loading (shared-memory, mmap) options.

However, there is a third data loading option, 'load directly', which is to directly load the datasets into the osrm-routed process memory.

The code paths for each data loading option are distinct:

This change adds 'load directly' as part of the Cucumber matrix configuration.
This will ensure future changes to dataset loading can be made without any regressions.

Impact

This increases the number of Cucumber tests run by 50%, but thanks to caching, the runtime increase is only 15%.

There is an opportunity to reduce the runtime. Some tests which are algorithm/data-loading agnostic now run six times, when once will suffice.

However, this requires tidying up the tags assigned to each test case, which feels like a bit of yak shaving I'd prefer to do after adding optional data loading.

Tasklist

Requirements / Relations

Closes #6661

Currently `npm test` runs the Cucumber suite with a matrix
configuration for selecting the algorithm (CH, MLD) and data loading
(shared-memory, mmap) options.

However, there is a third data loading option, 'load directly',
which is to directly load the datasets into the osrm-routed process memory.

The code paths for each data loading option are distinct:

Storage::Run + SharedMemoryAllocator
MMapMemoryAllocator
ProcessMemoryAllocator

This commit adds direct data loading as part of the Cucumber
configuration matrix.

This will ensure optional dataset support can be added without any
regressions.
Copy link
Member

@SiarheiFedartsou SiarheiFedartsou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@mjjbell mjjbell merged commit 522d0f0 into master Aug 1, 2023
@mjjbell mjjbell deleted the mbell/direct_load branch August 1, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add DirectLoader mode to default Cucumber test suite
2 participants